Skip to content

Put stone_removal_expiration onto AdHocClock #216

Merged
anoek merged 1 commit intomainfrom
fix_stone_removal_expiration_update
Dec 28, 2025
Merged

Put stone_removal_expiration onto AdHocClock #216
anoek merged 1 commit intomainfrom
fix_stone_removal_expiration_update

Conversation

@GreenAsJade
Copy link
Collaborator

…and use it to correctly calculate the stone removal time remaining.

(Fixes bug where clicking on the board during stone removal did not appear to reset the clock, so the clock appeared to count down to zero before it actually was at zero)

@github-actions
Copy link

Code Review

I've analyzed this PR and the changes look good overall. The fix correctly addresses the issue where the stone removal clock wasn't resetting properly by using the more specific stone_removal_expiration field when available.

Minor observation:

The stone_removal_mode field added to AdHocClock interface doesn't appear to be used anywhere in the codebase. While this might be intentional (for future use or documentation purposes), you may want to verify if it's needed as part of this change.

The logic using ?? operator is correct and maintains backwards compatibility when stone_removal_expiration is not set.

@GreenAsJade GreenAsJade requested a review from anoek December 27, 2025 10:09
@GreenAsJade
Copy link
Collaborator Author

GreenAsJade commented Dec 27, 2025

My Claude added the other field because technically this is what the server uses to indicate that the stone_removal_expiration is valid. Goban infers this indirectly instead as it happens. But the server is sending it, so it is in the JSON that's being described.

@anoek
Copy link
Member

anoek commented Dec 28, 2025

Awesome, thanks for digging into that and getting it fixed!

@anoek anoek merged commit cb09173 into main Dec 28, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants